Fix regex for apple-touch-icon #25025
Merged
+251
−15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
Contributor
|
WDYT about adding a handful of test cases for this? |
Contributor
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 29973 | |
| Version | PR #25025 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 396258a | |
| Installation URL | 4do3uvu533m8o |
Contributor
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 29973 | |
| Version | PR #25025 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 396258a | |
| Installation URL | 05r6qbt5f4ql8 |
Contributor
Author
Good idea. I generated a unit test suite for it. |
|
crazytonyli
reviewed
Nov 30, 2025
| let range = NSRange(location: 0, length: html.utf16.count) | ||
| if let match = regex?.firstMatch(in: html, options: [], range: range), | ||
| let matchRange = Range(match.range(at: 1), in: html), | ||
| let faviconURL = URL(string: String(html[matchRange]), relativeTo: siteURL) { |
Contributor
There was a problem hiding this comment.
I don't spot any real issue here, but I think the casting between Range and NSRange, String and NSString, and potentially unsafe subscript, could be avoided by using Swift.Regex.
crazytonyli
approved these changes
Nov 30, 2025
crazytonyli
pushed a commit
that referenced
this pull request
Nov 30, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 30, 2025
* Fix regex for apple-touch-icon (#25025) * Merge release_notes/26.5 into release/26.5 (#25036) * Update Jetpack release notes * Update metadata strings --------- Co-authored-by: Tony Li <[email protected]> * Update app translations – `Localizable.strings` * Update Jetpack metadata translations * Bump version number --------- Co-authored-by: Alex Grebenyuk <[email protected]> Co-authored-by: Tony Li <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes regression introduced in #25023. I should not have trusted AI to write regex.
Notes:
Before:
After (making the group actually optional):